Code Modernization: Use array_key_first() to read the first key of an array - #12790
Code Modernization: Use array_key_first() to read the first key of an array#12790Soean wants to merge 3 commits into
Conversation
… array. Reading the first key of an array through `array_keys()` builds a complete array of every key only to keep one entry and throw the rest away. Replace that with `array_key_first()`, which reads the first bucket directly.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
| $keys = array_keys( wp_match_mime_types( array_keys( $post_mime_types ), $post->post_mime_type ) ); | ||
| $type = reset( $keys ); | ||
| $type = array_key_first( wp_match_mime_types( array_keys( $post_mime_types ), $post->post_mime_type ) ); |
There was a problem hiding this comment.
wp_match_mime_types() returns an empty array when nothing matches, an attachment whose mime type falls outside the buckets get_post_mime_types() defines. Previously reset() gave false, and esc_attr( false ) is quiet. Now it's null, and esc_attr() → wp_check_invalid_utf8() → trim( null ), which is a deprecation notice on PHP 8.1+.
There was a problem hiding this comment.
Good catch on the empty array — pushed a guard in b645ef7.
One correction on the mechanism: wp_check_invalid_utf8() starts with (string) $text, so no trim() is involved and the escaping itself stays quiet. The actual leak is the last line of esc_attr(), which passes $text to the attribute_escape filter uncast — a plugin callback there would now get null where it used to get false.
$matched_types = wp_match_mime_types( array_keys( $post_mime_types ), $post->post_mime_type );
$type = array_key_first( $matched_types ) ?? '';I went through the other call sites too; this was the only one where the array can actually be empty and the result is used afterwards.
There was a problem hiding this comment.
Thanks for sharing details.
wp_match_mime_types() returns an empty array for an attachment whose mime type falls outside the buckets get_post_mime_types() defines. array_key_first() then yields null where reset() previously yielded false. The escaping chain itself is unaffected, as both wp_check_invalid_utf8() and _wp_specialchars() cast to string before anything else. But esc_attr() passes the raw value on to the 'attribute_escape' filter as its second argument, documented as string. Core hooks nothing there, while a plugin callback handing that argument to an internal string function would hit the PHP 8.1+ null deprecation that false did not trigger. Default to an empty string, and split the expression so the guard stays visible instead of trailing a 120-character line.
c321a9a to
b645ef7
Compare
2 participants
Reading the first key of an array through
array_keys()allocates a full array of every key just to keep one entry and discard the rest.array_key_first()reads the first bucket directly.Follow-up to 65773, which is scoped to the two nested
current( array_keys( $array ) )call sites and is addressed in #12785. This PR covers a second shape of the same idea, which that ticket does not include: the key array is assigned to a variable first, then read on the next line.13 occurrences across 11 files, in two shapes —
reset( $keys )and$keys[0]. In every case the intermediate variable existed only to carry the key array to the next line and is never read again afterwards.One change that goes further
In
spawn_cron()and_wp_cron()the surrounding check is dropped too:Both functions return early a few lines above when
$cronsis empty, soisset( $keys[0] )can never be false there. Happy to keep an explicitnull !== $firstguard instead if reviewers prefer the defensive form.Behavior notes
reset()returnsfalsewhilearray_key_first()returnsnull. None of the touched call sites compares the result with===, and most sit behind anempty()guard.$keys[0], the new code is strictly safer:$keys[0]emitted a notice on an empty array,array_key_first()returnsnullquietly.